Skip to content
This repository was archived by the owner on Jan 26, 2019. It is now read-only.

fix: TypeError in production build when targeting ES2015 #377

Merged
merged 1 commit into from
Aug 28, 2018
Merged

fix: TypeError in production build when targeting ES2015 #377

merged 1 commit into from
Aug 28, 2018

Conversation

ppvg
Copy link
Contributor

@ppvg ppvg commented Aug 6, 2018

Fixes #346.

This is a workaround for a bug in uglifyjs2, which can cause name collisions when a function with arguments is inlined. This can result in an unintended shadowing of a var or let, or a TypeError: Assignment to constant variable in case of a const.

I reproduced the issue by creating a new app (create-react-app --scripts-version=react-scripts-ts), changing tsconfig.json like so:

-    "target": "es5",
-    "lib": ["es6", "dom"],
+    "target": "es6",

...and running yarn build && npx http-server build.

I then confirmed the fix works by using my local build of react-scripts-ts:

   "dependencies": {
     "react": "^16.4.2",
     "react-dom": "^16.4.2",
-    "react-scripts-ts": "2.17.0"
+    "react-scripts-ts": "../create-react-app-typescript/packages/react-scripts"

...and running yarn && yarn build && npx http-server build.

Note: this fix increased the boilerplate's un-gzipped JS bundle size from 115969 to 116059 bytes.

@ppvg
fix: don't let uglify-es inline functions with arguments
aadae6a 
Fixes #346.

This is a workaround for a bug in uglifyjs2
(mishoo/UglifyJS#2842), which can cause name
collisions when a function with arguments is inlined. This can cause an
unintended shadowing of a `var` or `let`, or a `TypeError: Assignment to
constant variable` in case of a `const`.
@ppvg ppvg changed the title fix: don't let uglify-es inline functions with arguments fix: TypeError in production build when targeting ES2015 Aug 8, 2018
@ppvg
Copy link
Contributor Author

ppvg commented Aug 8, 2018 •
edited
Loading

The tests are failing because of SecurityError: localStorage is not available for opaque origins. I haven't looked into why that is. At first glance it seems unrelated. Perhaps someone more familiar with the build system could have a look?

@DorianGrey
Copy link
Collaborator

Sorry for the delay - just added a fix to get the CI working again, and restarted this PR's build.
The fix sounds legit, and I'll pick it up once the build is ready.

Side-note: CRA and CRA-TS are using uglifyjs-webpack-plugin, which uses uglify-es. That seems no longer maintained - maybe terser and the corresponding plugin might be favorable; it already includes several fixes, yet I'm not sure if one for the inlining feature is included.

@DorianGrey DorianGrey merged commit 8b5b8ae into wmonk:master Aug 28, 2018
@ppvg
Copy link
Contributor Author

ppvg commented Aug 28, 2018

Thanks!

maybe terser and the corresponding plugin might be favorable

Ah, interesting! Related: facebook/create-react-app#4711. I might look into this if I have some time.

@baophucct baophucct mentioned this pull request Nov 27, 2023
Open
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Uncaught TypeError when running production code locally after targeting ES2015 in tsconfig.json
2 participants
@ppvg @DorianGrey